automatically debug-log upstream http-response-sizes #1054
Conversation
gabor
force-pushed
the
gabor/log-response-info
branch
from
b4f83e8
to
dbedd92
Compare
|
@wbrowne @marefr could you please take a look at this draft? it's based on our discussion about logging "upstream" response-sizes... this is not polished yet, i'd just like to know if i'm going in the right direction.. notes:
in general, WDYT? thanks! |
There was a problem hiding this comment.
I think this make sense with some minor changes.
After reviewing the code I just realized that we already have something like this in core grafana, see https://github.com/grafana/grafana/blob/main/pkg/infra/httpclient/count_bytes_reader.go and https://github.com/grafana/grafana/blob/ddee95cb6d737ba845380c89456ede9998e12f48/pkg/infra/httpclient/httpclientprovider/datasource_metrics_middleware.go#L115-L119. So I think it would make sense to move the count_bytes_reader.go to the SDK instead replacing your ReportSizeReader
| if hasDatasourceTypeLabel { | ||
| rt = newReportSizeRoundtripper(rt) | ||
| } |
There was a problem hiding this comment.
You could put this logic in the middleware as well, see this example to get a hold of the options
. And I think then adding the middleware via DefaultMiddlewares() would allow both and http_client.go to support this.One important thing is that this middleware should probably/maybe execute after the https://github.com/grafana/grafana-plugin-sdk-go/blob/main/backend/httpclient/max_bytes_reader.go so if a limit is in place your middleware isn't continuing to read the response to the end 🤷
One thing to consider might be to somehow share the response size reader between limit and reporting middlewares, but maybe something for the future.
gabor
force-pushed
the
gabor/log-response-info
branch
from
dbedd92
to
750bca5
Compare
work in progress.